AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge

What was reviewed

Skipped (out of scope): dist/, pnpm-lock.yaml, binary PNGs

Findings

⚠️ Low — usePreviewSrc mutates state during render (rule-of-hooks violation)

File: src/hooks/usePreviewSrc.ts lines 463–469
Problem: The comparison prevUri.current !== committedUri is evaluated and setPreviewSrc(undefined) is called directly in the function body, not inside a useEffect. This is a React rule-of-hooks violation: calling a state setter during render is only permitted in a tightly constrained "derived state" pattern, and here it triggers a second render on every uri change, which can surprise strict-mode React and cause double-invocation warnings. The pattern also calls revoke() (which touches a ref) during render, violating the "render must be pure" contract.

Fix: Move the uri-change side effect into a useEffect:

useEffect(() => {
    if (prevUri.current !== committedUri) {
        prevUri.current = committedUri;
        if (blobRef.current) {
            revoke();
            setPreviewSrc(undefined);
        }
    }
}, [committedUri, revoke]);

⚠️ Low — crossOrigin removed from crop image without explanation; re-adds CORS risk for remote images

File: src/components/CropArea.tsx line 174 (diff context: removal of crossOrigin="anonymous")
Problem: The prior version had crossOrigin="anonymous" on the <img>. Removing it means that for remote images the browser serves them without a CORS exchange, and canvas.toBlob() will fail with a tainted-canvas SecurityError when the image is on a different origin. safeImageUri blocks javascript: URIs but does not ensure CORS compliance — it only validates the scheme.
Fix: Either restore crossOrigin="anonymous" (with the safeImageUri guard already in place, this is safe for all allowed schemes), or document in a code comment why it was removed and how callers are expected to serve images with the right headers.

⚠️ Low — Reset button missing aria-label; relies solely on visible text

File: src/components/CropToolbar.tsx line 272
Problem: The rotate buttons use aria-label (good), but the Reset and Grayscale buttons use only visible text with a title tooltip. The title attribute is not reliably announced by screen readers. For consistency with the rotate buttons and the accessibility guidelines, every toolbar button should have an explicit aria-label.
Note: The Grayscale button also has aria-pressed, which is correct for a toggle — this is just about making the accessible name explicit so it doesn't depend on title.

⚠️ Low — Missing CHANGELOG entry for new runtime behavior

File: packages/pluggableWidgets/image-cropper-web/CHANGELOG.md (not changed)
Problem: Rotation, grayscale, reset, zoom-anchor fix, and multi-instance crop isolation are all user-visible behavioral additions. Per repo convention a CHANGELOG entry is required when runtime code or widget behavior changes. Version bump is out of scope, but the unreleased entry should be present.
Fix: Run pnpm -w changelog and add an entry under [Unreleased] documenting the new features.

⚠️ Low — StaticCropPreview uses createRef instead of useRef inside a function component

File: src/ImageCropper.editorPreview.tsx line 28
Problem: createRef creates a new ref object on every render. Inside a function component, useRef should always be used because it returns the same ref across re-renders.
Fix:

- const imageRef = createRef<HTMLImageElement>();
+ const imageRef = useRef<HTMLImageElement>(null);

Positives

• safeImageUri blocks javascript:, vbscript:, and data:text/html URIs as defense-in-depth — security-conscious addition with corresponding tests covering all disallowed schemes.
• onUserInteractStart / userDraggedRef pattern cleanly prevents layout-driven onCropComplete from auto-committing; the multi-instance test proves sibling isolation explicitly.
• computeSourceRect is a single authoritative source for zoom-anchor math shared by cropImage and PreviewPane, ensuring the live preview and exported pixels agree — a subtle but important correctness property.
• useOriginalImage correctly guards against concurrent fetch races with the cancelled flag and skips re-capture of the widget's own baked outputs via markInternalChange — well-modeled lifecycle.
• usePreviewSrc revokes blob URLs on both uri-change and unmount, preventing memory leaks.
• Test coverage is extensive: 103 tests covering reset, grayscale reversibility, multi-instance isolation, zoom-anchor math, and stray-crop guard.
• Icon-only rotate buttons correctly use aria-label + title and <img alt=""> so the SVG is decorative — good accessibility hygiene.